Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[react_native_pods.rb] Introduce a reusable resolve_node_module function #41990

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kraenhansen
Copy link
Contributor

@kraenhansen kraenhansen commented Dec 19, 2023

Summary:

The React Native project and its related packages ship Ruby code, through packages published on NPM.
To successfully resolve the path of these "node modules", the project has historically made assumptions on the relative location of these packages. This however breaks down in the context of modern package managers and features such as NPM / Yarn workspaces, since packages may be (partially) hoisted to a parent / root package.

In recent times we've made changes to some of these Ruby files that makes it easier to consume in a hoisted environment: #36485

I suggest we generalise this by introducing a function, which the other scripts, developers apps and library authors can rely on to successfully resolve a path using the node modules resolution algorithm.

As an alternative, I've considered adding this as a function on the ReactNativePodsUtils module, but I felt this is such a versatile helper that it should be globally available. I'd be happy to change this if deemed necessary.

I'd love to follow up with additional PRs, introducing use of this utility function in more of the Ruby files of the project and related packages.

Changelog:

[IOS] [ADDED] - Add a resolve_node_module function to the react_native_pods.rb script, which will call Node.js to resolve a module path using the node modules resolution algorithm.

Test Plan:

I've made the change locally and tested that the bundle exec pod install command works as expected and verified that the resolve_node_module is available and works as expected from the .podspec of a library that has been installed into the React Native app that I'm testing with.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Dec 19, 2023
@robhogan robhogan self-assigned this Dec 19, 2023
@facebook-github-bot
Copy link
Contributor

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@react-native-bot react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 13, 2024
@kraenhansen
Copy link
Contributor Author

It looks like main diverged too much for this to be relevant, although I still see lots of ../node_modules in the files (which doesn't embrace the node modules resolution algorithm). What do you think @robhogan? You were also cooking something in this area if I remember correctly.

@react-native-bot react-native-bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants